fix: prevent division by zero when pageSize is 0 or negative#8271
fix: prevent division by zero when pageSize is 0 or negative#8271appfast-codev wants to merge 1 commit into
Conversation
|
Please provide an Online Example to show your problem. Thanks! |
There was a problem hiding this comment.
Pull request overview
Guards pagination calculations against invalid pageSize values to prevent division-by-zero during totalPages computation in the core pagination module.
Changes:
- Treat
pageSize <= 0as “show all rows” during pagination initialization. - Set
pageSizetototalRowsand mark the “all rows” option as selected for rendering.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (opts.pageSize <= 0) { | ||
| opts.pageSize = opts.totalRows | ||
| allSelected = true | ||
| } |
There was a problem hiding this comment.
The pageSize <= 0 normalization is inside if (opts.totalRows), but pageFrom/pageTo are calculated later unconditionally using opts.pageSize. If totalRows is 0 (or otherwise falsy) and pageSize is 0/negative, this leaves an invalid page range (e.g. pageFrom becomes 1 while pageTo can be 0/negative), which can leak into pagination info rendering (notably when onlyInfoPagination is enabled, the pagination container isn't auto-hidden for empty data). Consider normalizing opts.pageSize (or using a local effective page size) before the opts.totalRows check, and ensure arithmetic uses a positive page size even when totalRows is 0.
| if (opts.pageSize <= 0) { | ||
| opts.pageSize = opts.totalRows | ||
| allSelected = true | ||
| } |
There was a problem hiding this comment.
This change fixes a crash/NaN behavior for pageSize <= 0, but there doesn't appear to be a regression test covering pageSize set to 0 or a negative value. Adding a Cypress test case that initializes a table with pagination: true and pageSize: 0 (and -1) and asserts pagination renders without errors and behaves like “all rows” would help prevent this from regressing.
|
Thanks for the review! Here are some suggestions: 1. Move the guard before The current guard only normalizes if (opts.pageSize <= 0) {
opts.pageSize = opts.totalRows || 1
}Note the 2. Add A if (opts.pageSize <= 0) {
console.warn('pageSize must be a positive number, falling back to show all rows.')
opts.pageSize = opts.totalRows || 1
} |
|
Thanks for the PR! I've addressed the review feedback and created #8294 with the following improvements based on your work:
I'll close this PR in favor of #8294. Thanks again for the contribution! |
|
Closing in favor of #8294 which includes the fix with additional improvements. |
In initPagination, if pageSize is 0 or negative, the calculation ~~((totalRows - 1) / pageSize) + 1 causes a division by zero. Added a guard to treat pageSize <= 0 the same as 'show all rows', which sets pageSize to totalRows and allSelected to true. All 339 existing tests pass.